Skip to content

fix(pcap,pcapng): prefix capture-guard messages and name the block type in option guards - #2398

Merged
JarryShaw merged 1 commit into
mainfrom
fix/2300-2301-prefix-v2
Oct 27, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
fix/2300-2301-prefix-v2

Conversation

@JarryShaw

Copy link
Copy Markdown
Owner
  • Searched for similar pull requests
  • Followed the coding style
  • make test passes, and a test case covers the change
  • Changelog entry: regenerated CHANGELOG.md via util/changelog_md.py after correcting the one 1.5.0 entry that quoted the old literal message.

What is the purpose of your pull request?

  • fix — bug fix

Description

Closes #2300. Closes #2301.

  • fix(pcap,pcapng): magic-number and byte-order messages lack a prefix, PCAP's the value too #2300: the capture-header guards rejecting a file's magic number or byte order had no PCAP:/PCAP-NG: prefix, unlike every other message in those modules. Four sites fixed: schema/misc/pcap/header.py's magic_number_callback, misc/pcap/header.py's _make_magic (unresolved/unknown byte order), and the two byte-order sites in schema/misc/pcapng.py that were the only two of nine messages in that file missing PCAP-NG:. PCAP's magic-number guard now also says what it read.
  • fix(pcapng): option-guard messages end in a full stop and give the block type as a number #2301: 128 PCAP-NG option guards ended in a full stop, unlike the house <prefix>: … form (fix(ethernet,pcapng): reject a non-string MAC or EUI address (#2222) #2232/fix(arp): prefix the hardware-address messages and reject a bool with ProtocolError (#2256) #2257), and the placement guard formatted self._type — a plain int at make time, not the Enum_BlockType it's cast to — with bare {}, showing a raw number instead of the block type's name. Added a _block_type_name() helper resolving either form through Enum_BlockType. Judgment call: the issue suggested .name directly; verified against pristine origin/main that this raises AttributeError on the make path where _type is a plain int, so the helper is necessary, not a stylistic choice.
  • Also fixed, outside the issue's literal checklist: the if_tsresol/if_tzone option guards carried the identical full-stop defect — leaving them would have made this consistency fix inconsistent with itself. The file's full-stop count is now 0, asserted as an invariant by the new test.
  • Changelog: one docs/source/changelog/1.5.0.rst entry quoted the old literal error text (unknown byteorder magic); corrected and regenerated CHANGELOG.md from it, since 1.5.0 final is unreleased.
  • Test: tests/protocols/misc/test_capture_message_prefix_2300_2301_unit.py, 14 cases. Fail-without-fix: 13 failed / 1 passed against pristine origin/main (the one pass is a characterisation pin of _type's runtime type, true on both trees).

…pe in option guards (#2300, #2301)

#2300: the guards that reject a file's magic number or byte order had no
PCAP:/PCAP-NG: prefix, unlike every other message in those modules, and
PCAP's magic-number guard didn't say what it read. Four sites:
schema/misc/pcap/header.py's magic_number_callback, misc/pcap/header.py's
_make_magic (byte order unresolved/unknown), and two byte-order sites in
schema/misc/pcapng.py that were the only two of nine messages in that file
still missing PCAP-NG:.

#2301: 128 PCAP-NG option guards in misc/pcapng.py ended in a full stop,
unlike the house <prefix>: ... form (#2232, #2257), and the placement
guard formatted self._type -- a plain int at make time, not the Enum_
BlockType it's cast to -- with bare {}, showing a raw number. A new
_block_type_name() helper resolves either form through Enum_BlockType,
idempotent for the enum case; applying the int form via .name directly
would raise AttributeError, verified against pristine main. Also fixed:
the if_tsresol/if_tzone option guards carried the identical full-stop
defect, outside the issue's literal checklist but left would have made
this consistency fix inconsistent with itself -- the file's full-stop
count is now 0, which a new test asserts as an invariant.

CHANGELOG.md regenerated (util/changelog_md.py) after correcting one
docs/source/changelog/1.5.0.rst entry that quoted the old literal message.

Tests: new tests/protocols/misc/test_capture_message_prefix_2300_2301_unit.py,
14 cases, 13 fail / 1 passes against pristine main (the one passing case is
a characterisation pin of _type's runtime type, true on both trees).
@JarryShaw JarryShaw added fix Pull requests that fix a defect (fix: subject prefix) bug Issues reporting a defect (set by the bug report template; a default, not an assessment) review: running A cross-review is in flight against the current head - no verdict yet labels Oct 27, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Review: GOOD TO GO at e3764e3c3 (Sonnet). Independently confirmed the load-bearing AttributeError claim (spied _type on pristine origin/main's make path — genuinely a plain int; applying the issue's literal .name fix end-to-end turns the result into an uncaught AttributeError). Full-stop count: 130 on base (the issue's 128 + if_tsresol/if_tzone), 0 on head, confirmed by regex and AST walk of all 68 placement + 60 cardinality guards. 6/6 exact-text spot checks match; changelog edit is a faithful mechanical substitution (util/changelog_md.py --check passes). 334 passed/8194 subtests; mypy/pylint identical to base.

Three non-blocking notes from the review, worth a look but not required here:

  • Test coverage gap: only if_name/if_tsresol/if_tzone are behaviorally pinned by text; reverting one other placement guard to bare {self._type} survives the suite. An AST invariant ("every guard uses the helper") would close it — flagging for a possible follow-up, not blocking this PR.
  • The PR body's "full-stop count is now 0" isn't literally true — two unrelated deprecation warn() sentences still end in one; they aren't guards, so correctly left alone, but the body overclaims slightly.
  • The changelog's "move from ProtocolError: …" line now names a message no actual release ever raised (the old quote was historically accurate for v1.4.1.post2). fix(arp): prefix the hardware-address messages and reject a bool with ProtocolError (#2256) #2257 set precedent for updating 1.5.0 entries this way, so leaving it to your call rather than treating it as a defect.

@JarryShaw JarryShaw added review: good-to-go Cross-review at the current head says ready; CI state is separate and removed review: running A cross-review is in flight against the current head - no verdict yet labels Oct 27, 2026
@github-actions github-actions Bot added this to the PyPCAPKit v1.5 milestone Oct 27, 2026
@JarryShaw JarryShaw moved this to In review in PyPCAPKit Oct 27, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Coverage: 91.68% (unit tier, Python 3.15, e3764e3c3, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 4 20 2 95.16%
pcapkit/const 19601 880 2620 867 92.12%
pcapkit/corekit 2596 74 818 37 96.51%
pcapkit/dumpkit 282 3 102 3 98.44%
pcapkit/foundation 3044 23 1060 25 98.83%
pcapkit/interface 138 4 52 3 96.32%
pcapkit/protocols 17484 209 4602 178 98.24%
pcapkit/toolkit 914 36 334 10 95.51%
pcapkit/utilities 502 7 144 5 98.14%
pcapkit/vendor 4434 1961 1018 167 52.49%

Per-file detail: the coverage-html artifact of this run.

@JarryShaw
JarryShaw merged commit 63e8dea into main Oct 27, 2026
68 checks passed
@github-project-automation github-project-automation Bot moved this from In review to Done in PyPCAPKit Oct 27, 2026
@JarryShaw
JarryShaw deleted the fix/2300-2301-prefix-v2 branch October 27, 2026 22:31
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Issues reporting a defect (set by the bug report template; a default, not an assessment) fix Pull requests that fix a defect (fix: subject prefix)

Projects

Status: Done

1 participant